All committers have signed the CLA.

Pull Request Test Coverage Report for Build 16423372178

Details

• 704 of 768 (91.67%) changed or added relevant lines in 51 files are covered.
• 11 unchanged lines in 3 files lost coverage.
• Overall coverage decreased (-0.5%) to 90.0%

Totals
Change from base Build 16361074893:	-0.5%
Covered Lines:	6561
Relevant Lines:	7290

💛 - Coveralls

FYI. I merged a PR that fixed CI with the main branch and changes that will going into the shortly to be released 1.2.0 so that CI now passes fully here and does not fail with the two jobs that check things out against qiskit main, and updated the branch here so this now completely passes.

I think this is fine. I must admit though I had wondered, given that I think we only ever need something that has a run() method, that takes circuits and gives transpiled circuits back whether, something could be done around a type that was more targeted along those lines. The pass manager has a number of methods that I do not think we would call, and the AI Transpiler Service which has a run method could be used too but is not a passmanager - i,e, so that a Passmanager instance or the AI transpiler could be used - or anything else that had a run method that did the transpilation. I know Optimization went for BassPassManager too as the type and maybe thats ok.

Whatever is done I think we might want to note in the docstring for this that's its optional and that passing in None results in the circuit not being transpiled. Maybe for some algos like VQE one can already pass a circuit (ansatz) that is transpiled for a given backend so the note there might be a little different to accommodate that - for PE they build their own circuits of course so that aspect does not apply.

One minor comment I might have is that the pm used in the tests is created and ends up being used for all the tests rather than being created at the time the data is passed into the test. As long as there are no side-effects of using pm, i.e. its the same really as creating a new pm for each test run I guess its ok - my feeling is that it would be better to do it each time so it guarantees a fresh instance is used.

And another weird thing: the CLA complained that some commits done by Léna weren't associated to an email address that signed the CLA when the PR was still a draft, but doesn't seem to complain now that we've put it ready for review, is it expected?

I think the CLA is for qiskit projects overall so could they have signed it on another one. If CLA passes it all seems good to me.

@woodsp-ibm We've added the release note, let us know if that's not what you expected!

I am thinking that perhaps adding a link to this migration guide in the release notes might be helpful https://quantum.cloud.ibm.com/docs/en/migration-guides/v2-primitives It not only covers the runtime ones but also has a link to the v1 V2 migration for reference primitives in Qiskit.

Do you think an Estimator sample of before with V1 and now with V2 would be helpful as well, alongside the Grover Sampler one. That would cover both primitive types and their usage in algorithms from a migration perspective.

One minor text item

Replace all instances of V1 primitives by their V2 counterpart.

Would this be better i.e. usage versus instances

Replace all usage of V1 primitives by their V2 counterpart.

Overall though it seems fine to me.

We've updated the release note and also removed a section in another release note stating that qiskit-algorithms wasn't supporting Qiskit 2.0, since this is now the case.

@woodsp-ibm I'm not sure to understand the reason why the make html build fails. It seems like Sphinx complains about a duplicate description for the ansatz attribute of VQE, VQD and PVQD but we're not too sure about what we should do about it. Do you have any guidance on this?

Apart from that, the only things that are left to do are adding a transpilation option to AdaptVQE, which is linked to the discussion in #231, and manually circumventing the Qiskit issues that aren't going to be fixed, as per the discussion within them.

It seems like Sphinx complains about a duplicate description for the ansatz attribute of VQE, VQD and PVQD but we're not too sure about what we should do about it. Do you have any guidance on this?

ansatz was a public attribute in those classes right, and you made it private and added an explicit getter and setter which both have docstrings on them. Before - and its still there - such public attributes were described in the Attributes: section that precedes the init method. Given ansatz is now done via the documented getter/setter I think deleting the entry from the list in Attributes: should fix things.

It seems its still failing the same with ansatz as it looks like you removed anstaz from the Args section rather than the Attributes section. It should be in Args, as its a parameter to the init constructor, just not in Attributes any more. The Attributes section is just above the init methid describing public attribues of the class. E.g. line 94 in your vqd.py

It seems its still failing the same with ansatz as it looks like you removed anstaz from the Args section rather than the Attributes section. It should be in Args, as its a parameter to the init constructor, just not in Attributes any more. The Attributes section is just above the init methid describing public attribues of the class. E.g. line 94 in your vqd.py

Yeah, not my brightest moment :')

@woodsp-ibm By the way, there's a test in in test_sampler_gradient.py (at line 595 currently) that we skipped because of the time it was taking to run. However, we're not too sure about how relevant this test is in the first place. Should we remove the skip, which would quite drastically increase the tests running time, or should we remove the test altogether?

Should we remove the skip, which would quite drastically increase the tests running time, or should we remove the test altogether?

I would leave things like they are for now i.e. skipping the test. You have a comment there it seems that it takes some time. I am not sure about the test normally we expected tests to be fast but things have changed quite a bit since that code was originally done.

I do not know the rationale for that test - @a-matsuo I think you were main author (looking back at the history) would you have any input here.